-
Notifications
You must be signed in to change notification settings - Fork 13
Fix error with setting repeated proto field for bundling. #111
Conversation
@tbetbetbe I know that the pep8 and pylint tests currently fail. They just need to ignore the generated fixture_pb2.py file. Otherwise this is ready to review. |
Update bundling tests to use actual proto messages, to prove this fix works.
|
||
[testenv:pylint-errors] | ||
setenv = | ||
PYTHONPATH = {toxinidir}:{toxinidir}/test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this because we weren't linting the test/
directory previously?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This wasn't really needed. I got rid of this change.
Current coverage is 97.16%@@ master #111 diff @@
==========================================
Files 8 8
Lines 600 599 -1
Methods 0 0
Messages 0 0
Branches 0 0
==========================================
- Hits 583 582 -1
Misses 17 17
Partials 0 0
|
Woot! Woot! I finally got a passing build with protoc / Travis CI / tox. :) |
LGTM |
Note: this adds an implicit dependency of bundling.py on proto, namely the ClearField method. Ideally this should be moved into gRPC.py and called through the config.py "interface", but that can be done as a follow-up. |
Added TODO comment to code based on feedback |
Update bundling tests to use actual proto messages, to prove this fix
works.
Fixes #110